Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement ixmp4 shim #516

Closed
wants to merge 10 commits into from
Closed

Implement ixmp4 shim #516

wants to merge 10 commits into from

Conversation

khaeru
Copy link
Member

@khaeru khaeru commented Jan 25, 2024 •
edited
Loading

Superseded by #552 with a renamed branch.

This PR is intended to aid @glatterf42 in developing model data features in ixmp4. It is not a complete plan or solution for migrating ixmp-based code, workflows, and research to ixmp4; such a plan would involve many additional elements that are not in scope for this PR.

The PR:

  • Adds a IXMP4Backend class that allows ixmp.Platform, ixmp.TimeSeries, and ixmp.Scenario (thus all existing code based on ixmp and message_ix) to talk to ixmp4's API. This functions as a shim between the established and new/developing APIs.
  • Makes minimal changes to allow ixmp.Platform backed by IXMP4Backend to be created and used.
  • Adjusts or parametrizes tests of the ixmp Platform, TimeSeries, and Scenario classes to run against both JDBCBackend and IXMP4Backend, ensuring the latter provides the same behaviour expected of the former per the test suite.
    • This is a precondition for migrating other code (for instance, in message-ix-models or message_data) to, first, use ixmp4 via an IXMP4Backend, and eventually, ixmp4 directly.
  • May serve as a basis for a parallel PR in message_ix.

Notes

  • ixmp4 is not published on PyPI for Python 3.8 and 3.9 and can't be installed in those CI jobs. Possible ways to respond:
    • Separate ixmp4 from the tests dependency, and don't install it on those Pythons.
    • Adjust parametrized tests to not import ixmp4 or run against IXMP4Backend with those Pythons.
    • Document that IXMP4Backend is only available for Python >= 3.10.

How to review

TBD. This branch/PR may not necessarily be merged itself, but may serve as the basis for another PR that adds the IXMP4Backend.

PR checklist

  • Continuous integration checks all ✅
  • Add or expand tests; coverage checks both ✅
  • Add, expand, or update documentation.
  • Update release notes.

@khaeru khaeru added the enh New features & functionality label Jan 25, 2024
khaeru
khaeru commented Jan 25, 2024
View reviewed changes
ixmp/backend/ixmp4.py
# WHERE run.is_default = 1 ORDER BY run.id ASC]
return self._platform.runs.list()

# The below methods of base.Backend are not yet implemented
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @glatterf42, the idea is that to remove lines like name = _ni below as each method gets a concrete implementation, in parallel with tests that exercise it.

khaeru
khaeru commented Jan 25, 2024
View reviewed changes
ixmp/tests/core/test_platform.py
def test_getattr(self, test_mp):
"""Test __getattr__."""
with pytest.raises(AttributeError):
test_mp.not_a_direct_backend_method

def test_scenario_list(self, mp):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently fails (e.g. here) with:

sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) no such table: run
[SQL: SELECT DISTINCT run.model__id, run.scenario__id, run.version, run.is_default, run.id 
FROM run 
WHERE run.is_default = 1 ORDER BY run.id ASC]

@khaeru
Copy link
Member Author

khaeru commented Jan 25, 2024

The Python 3.12 job fails with different exceptions than 3.10 and 3.11, for instance here:

ixmp/backend/ixmp4.py:15: in __init__
    import ixmp4
[...]
/opt/hostedtoolcache/Python/3.12.1/x64/lib/python3.12/site-packages/sqlalchemy/orm/decl_api.py:303: in __get__
    reg[self] = obj = self.fget(cls)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

cls = <class 'ixmp4.data.db.unit.model.Unit'>

    @declared_attr.directive
    def columns(cls):
>       return cls.__table__.columns
E       AttributeError: type object 'Unit' has no attribute '__table__'. Did you mean: '__le__'?

/opt/hostedtoolcache/Python/3.12.1/x64/lib/python3.12/site-packages/ixmp4/data/db/base.py:76: AttributeError

I guess this means ixmp4 does not support Python 3.12, although it is installable.

@khaeru khaeru force-pushed the feature/ixmp4-backend branch from e6624c2 to 92ee78d Compare January 30, 2024 08:39
@khaeru khaeru force-pushed the feature/ixmp4-backend branch from 92ee78d to 3e38bf0 Compare March 1, 2024 07:34
@khaeru khaeru mentioned this pull request Apr 23, 2024
Merged
2 tasks
@khaeru khaeru mentioned this pull request Aug 12, 2024
Draft
6 tasks
@khaeru khaeru force-pushed the feature/ixmp4-backend branch from 3e38bf0 to 0c9022f Compare November 28, 2024 09:27
khaeru added a commit to iiasa/message_ix that referenced this pull request Nov 28, 2024
@khaeru
Use branch for iiasa/ixmp#516 in CI
e487b67
@khaeru khaeru mentioned this pull request Nov 28, 2024
Draft
7 tasks
@khaeru khaeru changed the title Implement IXMP4Backend Implement ixmp4 shim Nov 28, 2024
@khaeru khaeru closed this Nov 28, 2024
@khaeru khaeru deleted the feature/ixmp4-backend branch November 28, 2024 10:08
@khaeru khaeru mentioned this pull request Nov 28, 2024
Draft
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enh New features & functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@khaeru